Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Finalize migration of reporting interfaces #71

Merged
merged 6 commits into from
Oct 5, 2024

Conversation

oesteban
Copy link
Member

@oesteban oesteban commented Apr 18, 2023

This PR brings generalistic "reportlet capable" interfaces from niworkflows. It doesn't port those interfaces modified by niworkflows (e.g., the report-generating version of the normalization interface).

I'm a bit on the fence about this. Perhaps a cleaner way is to have a nipreps.interfaces namespace where packages can "contribute" their custom interfaces, where a set of default interfaces is available.

This would be consistent with @mgxd's idea of nipreps.synthstrip and so forth.

Also, splitting workflows and interfaces from niworkflows would probably be the best approach for clarity, and have niworkflows populate nipreps.workflows and nipreps-interfaces (or similar) populate nipreps.interfaces.

What do you think?

Resolves: #54.

@oesteban oesteban force-pushed the enh/54-migrating-reportlet-interfaces-2 branch 2 times, most recently from b91ff7c to 7ea0b3a Compare April 18, 2023 08:18
@codecov-commenter
Copy link

codecov-commenter commented Apr 18, 2023

Codecov Report

Attention: Patch coverage is 54.25532% with 172 lines in your changes missing coverage. Please review.

Project coverage is 64.70%. Comparing base (6404af3) to head (cc88653).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
nireports/interfaces/reporting/registration.py 49.30% 72 Missing and 1 partial ⚠️
nireports/interfaces/reporting/segmentation.py 46.53% 54 Missing ⚠️
nireports/interfaces/reporting/masks.py 65.64% 40 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #71      +/-   ##
==========================================
+ Coverage   60.34%   64.70%   +4.35%     
==========================================
  Files          20       23       +3     
  Lines        2242     2618     +376     
  Branches      399      422      +23     
==========================================
+ Hits         1353     1694     +341     
- Misses        795      812      +17     
- Partials       94      112      +18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@oesteban oesteban force-pushed the enh/54-migrating-reportlet-interfaces-2 branch from 7ea0b3a to fb79a20 Compare April 18, 2023 08:27
@oesteban oesteban requested a review from effigies April 18, 2023 08:28
@oesteban oesteban force-pushed the enh/54-migrating-reportlet-interfaces-2 branch from fb79a20 to 6846c70 Compare April 18, 2023 08:41
@oesteban oesteban force-pushed the enh/54-migrating-reportlet-interfaces-2 branch from 6846c70 to 4182979 Compare May 19, 2023 06:45
oesteban added a commit that referenced this pull request May 25, 2023
This sidetracks #71 for interfaces with little discussion about whether
they can be consider core enough to be maintained within NiReports.

In addition, moves the SimpleBeforeAfter interface into the set of basic
interfaces, as it is very often used.
oesteban added a commit that referenced this pull request May 25, 2023
This sidetracks #71 for interfaces with little discussion about whether
they can be consider core enough to be maintained within NiReports.

In addition, moves the SimpleBeforeAfter interface into the set of basic
interfaces, as it is very often used.
Copy link
Contributor

@mgxd mgxd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should get this in and cut a new release, so we can:

  1. migrate end user tools away from niworkflows reporting
  2. deprecate niworkflows reporting

nireports/conftest.py Outdated Show resolved Hide resolved
@oesteban
Copy link
Member Author

@effigies @mgxd @esavary @celprov - if I forget to bring this up in the next TechMon, could you remind me?

@effigies effigies force-pushed the enh/54-migrating-reportlet-interfaces-2 branch from 8d197b5 to 70bfd85 Compare October 4, 2024 21:01
@effigies effigies merged commit 67b5f0c into main Oct 5, 2024
22 of 23 checks passed
@effigies effigies deleted the enh/54-migrating-reportlet-interfaces-2 branch October 5, 2024 01:18
effigies added a commit that referenced this pull request Dec 19, 2024
24.1.0 (December 18, 2024)

New feature release in the 24.1.x series.

This release includes a migration of most if not all reporting
interfaces from NiWorkflows.
This release also supports Python 3.13 and Numpy 2.

* ENH: Finalize migration of reporting interfaces (#71)
* ENH: Allow figures in session folder (#138)
* RF: Replace nireports.data.Loader with acres.Loader (#142)
* STY: Apply new ruff rules (#139)
* MAINT: Add tox.ini, test minimum dependencies (#141)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port niworkflows.interfaces.reportlets
4 participants